Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Thread safe cloexec #1828

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Thread safe cloexec #1828

wants to merge 2 commits into from

Conversation

kdombroski
Copy link

Thread safe socket creation, using:

  • SOCK_CLOEXEC on Linux (and others where available)
  • accept4 on Linux
  • WSAAccept on Windows

Karen Dombroski added 2 commits May 1, 2024 21:46
threadsafe CLOEXEC on platforms that support it
50fce53 
SOCK_CLOEXEC is a flag available on some platforms to enable creation of
sockets with CLOEXEC already set
threadsafe accept on windows, linux
fb739db 
* Windows has WSAAccept() which will create sockets inheriting flags from
  the server socket

* Linux has accept4() which has a flags argument supporting SOCK_CLOEXEC
@yhirose
Copy link
Owner

yhirose commented May 2, 2024

@kdombroski thanks for your pull request, but I am not sure if it is really needed. Could you explain why you need the changes and benefits? Thanks.

@kdombroski
Copy link
Author

There is a brief window of time between creating a socket and calling fcntl with FD_CLOEXEC where another thread could create a new process inheriting the file descriptor.
Adding the SOCK_CLOEXEC flag eliminates this race condition.

@yhirose
Copy link
Owner

yhirose commented May 2, 2024

How about the followings?

  • accept4 on Linux
  • WSAAccept on Windows

@kdombroski
Copy link
Author

accept4 on Linux and WSAAccept on Windows is great.
But - for true fork() safety sockets need to be constructed with the SOCK_CLOEXEC flag.

From the open manpage:
"Note that the use of this flag is essential in some multithreaded programs, because using a separate fcntl(2) F_SETFD operation to set the FD_CLOEXEC flag does not suffice to avoid race conditions where one thread opens a file descriptor and attempts to set its close-on-exec flag using fcntl(2) at the same time as another thread does a fork(2) plus execve(2)."

SOCK_CLOEXEC is supported on Linux and BSD.
The main platform without it is Apple - I have not found an alternative there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@kdombroski @yhirose